Skip to content

Support NullMarked and NullUnmarked Annotations - #26559

Open
HarrisL2 wants to merge 5 commits into
scala:mainfrom
HarrisL2:nullmarked-annot
Open

Support NullMarked and NullUnmarked Annotations#26559
HarrisL2 wants to merge 5 commits into
scala:mainfrom
HarrisL2:nullmarked-annot

Conversation

@HarrisL2

Copy link
Copy Markdown
Contributor

Based on #26456

Adds support for NullMarked and NullUnmarked. Inside Java source, symbols with NullMarked in scope will be non-null, and NullUnmarked scopes remain implicit.

Have you relied on LLM-based tools in this contribution?

Yes, and I checked the output by hand and writing the corresponding tests.

How was the solution tested?

New automated tests

Comment thread tests/explicit-nulls/neg/nullmarked-packageinfo-separate/S_1.scala
Comment thread tests/explicit-nulls/neg/nullmarked-packageinfo/S.scala
Comment thread tests/explicit-nulls/neg/nullmarked-separate/S_3.scala
Comment thread tests/explicit-nulls/neg/nullmarked/S.scala
Comment thread tests/explicit-nulls/neg/nullmarked-packageinfo-separate/J.java
Comment thread compiler/src/dotty/tools/dotc/core/ImplicitNullInterop.scala
Comment thread tests/explicit-nulls/neg/nullmarked-packageinfo/package-info.java Outdated
Co-authored-by: TheDrawingCoder-Gamer <TheDrawingCoder-Gamer@users.noreply.github.com>
Comment thread tests/explicit-nulls/neg/nullmarked-separate/J_2.java

@olhotak olhotak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM, but @noti0na1 should also have a look.

Comment thread compiler/src/dotty/tools/dotc/core/Definitions.scala

@TheDrawingCoder-Gamer TheDrawingCoder-Gamer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also LGTM

@noti0na1 noti0na1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core change looks good to me. The test suit need to be extended for:

  • sub packages;
  • a jar, if possible;
  • generics;
  • a package marked by both annotations.

* `@NullUnmarked` re-enables the implicit-nulls default.
*/
private def isNullMarked(sym: Symbol)(using Context): Boolean =
sym.ownersIterator.map(ownerNullMarking).collectFirst { case Some(marked) => marked }.getOrElse(false)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the document (https://jspecify.dev/docs/api/org/jspecify/annotations/NullMarked.html):

@NullMarked and @NullUnmarked ... To apply these annotations to an entire (single) package, create a package-info.java file and annotate the package declaration there. This annotation has no effect on "subpackages".

We may want to stop the iteration at package level.

* classes). For all other owners we use `unforcedAnnotation` to avoid forcing symbols that may
* still be under construction during classfile loading / unpickling.
*/
private def ownerNullMarking(owner: Symbol)(using Context): Option[Boolean] =

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to also handle module-info?

* @param state mutable nullification state tracking the current mode for the result type
* (`resultTypeMode`) and the current nesting level (`currentTypeMode`).
*/
private class ImplicitNullMap(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw the spec has some special rules for wildcards and type parameters. Are they supported by this PR? If not, I'm not sure if we want to support them?

@noti0na1 noti0na1 assigned HarrisL2 and unassigned noti0na1 Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants